Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: RoadRunner streaming through generators #939

Merged
merged 2 commits into from
Aug 5, 2024
Merged

fix: RoadRunner streaming through generators #939

merged 2 commits into from
Aug 5, 2024

Conversation

danharrin
Copy link
Contributor

Fixes #903.

RoadRunner supports streamed responses, but not in the same way that other servers do. Instead out using the output buffer with echo, it supports using a Generator function as the response callback to "yield" each chunk of the response.

Previous fixes have involved collecting the output buffer and returning it at once from the server. This is not streaming as all chunks are collected into memory and sent at once.

This implementation allows you to use a Generator as the callable in the streamed response. Each yield sends a chunk in the response. It is quite simple and just interacts with the underlying HttpWorker that is used by the Roadrunner client. The new code is only invoked if the streamed response function is a generator, else the old behaviour still persists.

use Generator;

return response()->stream(function (): Generator {
    yield 1;

    sleep(1);

    yield 2;

    sleep(1);

    yield 3;

    sleep(1);

    yield 4;

    sleep(1);

    yield 5;
}, 200, [
    'Content-Type' => 'text/html; charset=utf-8;',
    'Cache-Control' => 'no-cache',
    'X-Accel-Buffering' => 'no',
]);

Note: we do not need to evaluate the function to know if we need to stream it in this way, we use reflection to do this ahead of time, ensuring that the function returns a Generator object.

I would like to thank @donnysim for his investigative work and initial solution which was adapted into this PR.

if (
($octaneResponse->response instanceof StreamedResponse) &&
($responseCallback = $octaneResponse->response->getCallback()) &&
((new ReflectionFunction($responseCallback))->getReturnType()?->getName() === Generator::class)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be:

((new ReflectionFunction($responseCallback))->isGenerator()

a typehint would not be needed to know if it's a generator or not

@donnysim
Copy link

donnysim commented Aug 5, 2024

I'm so sorry, I completely forgot about the whole thing and could've saved you some trouble if you were too busy 🙏. This looks good though 👍.

@taylorotwell taylorotwell merged commit 2b5b4d0 into laravel:2.x Aug 5, 2024
12 checks passed
@danharrin danharrin deleted the fix/roadrunner-streaming branch August 5, 2024 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Streamed response gets buffered and returned in one chunk
3 participants